StableMIR: Implement `CompilerInterface` #139852

makai410 · 2025-04-15T08:01:17Z

This PR implements part of the document.

With TablesWrapper wrapped by CompilerInterface, the stable-mir's TLV stores a pointer to CompilerInterface, while the rustc-specific TLV stores a pointer to tables.

rustbot · 2025-04-15T08:01:22Z

r? @scottmcm

rustbot has assigned @scottmcm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

makai410 · 2025-04-15T08:08:22Z

r? @celinval btw should we create a tracking issue for this?

makai410 · 2025-04-15T08:20:21Z

compiler/rustc_smir/src/rustc_internal/mod.rs

@@ -233,7 +234,12 @@ where
        mir_consts: IndexMap::default(),
        layouts: IndexMap::default(),
    }));
-    stable_mir::compiler_interface::run(&tables, || init(&tables, f))
+
+    let interface = CompilerInterface { cx: tables };


I put the interface here since I'd prefer not to refactor the launch process now. It would be more appropriate to do this at the end.

celinval

Awesome!

celinval · 2025-04-15T22:58:55Z

compiler/rustc_smir/src/stable_mir/compiler_interface.rs

+/// Stable-MIR’s interface to compiler internals.
+/// All internal queries must go through [`Context`].
+///
+/// Do not use this directly.


Can you please document that this is currently used in the macro expansion?

hmmm tbh it is not. We directly create a CompilerInterface in the rustc_internal::run() function for now, which is also called by pretty::write_smir_pretty(), not only the run_driver!.

but yeah, maybe we can make it into a macro

Oh my bad! I got it wrong...

celinval · 2025-04-15T23:06:08Z

compiler/rustc_smir/src/stable_mir/compiler_interface.rs

-// A thread local variable that stores a pointer to the tables mapping between TyCtxt
-// datastructures and stable MIR datastructures
+/// Stable-MIR’s interface to compiler internals.
+/// All internal queries must go through [`Context`].


Can we remove Context in this PR?

Also, I think it may be worth documenting this a bit better, so people know why this is here. Something on the lines that this structure will serve as a bridge between StableMIR and the rustc_smir which will provide similar APIs but based on internal rustc constructs.

Can we remove Context in this PR?

Sorry that i didn't get it. You mean we directly write something like impl<'tcx> TablesWrapper<'tcx>, then move everything that used to be in Context to there?

Oh and then we create a brand-new Context in rustc_smir, and gradually move methods from TablesWrapper to it until TablesWrapper is completely hollowed out.

Yes, you don't need the trait anymore. Next step I believe would be to move the stable / internal calls out of the TablesWrapper.

Btw, you could rename TablesWrapper to Context too. ☺️

Also, feel free to improve the name scheme, it's not my strongest skill. LoL

hmmm what if we change CompilerInterface to SmirInterface?🤔

celinval · 2025-04-15T23:08:08Z

compiler/rustc_smir/src/stable_mir/compiler_interface.rs

 scoped_tls::scoped_thread_local!(static TLV: Cell<*const ()>);

-pub fn run<F, T>(context: &dyn Context, f: F) -> Result<T, Error>
+pub fn run<'tcx, T, F>(interface: &CompilerInterface<'tcx>, f: F) -> Result<T, Error>


@oli-obk Any chance you can take a look at this TLV magic? I could use a second pair of eyes here. Thanks!

celinval · 2025-04-18T05:21:46Z

compiler/rustc_smir/src/rustc_smir/context.rs

+pub(crate) struct Context<'tcx>(pub RefCell<Tables<'tcx>>);
+
+impl<'tcx> Context<'tcx> {
+    pub(crate) fn target_info(&self) -> MachineInfo {


You might as well make these public so we don't have to update the visibility when we pull the StableMir stuff back to its crate

- With `Context` wrapped by `SmirInterface`, the stable-mir's TLV stores a pointer to `SmirInterface`, while the rustc-specific TLV stores a pointer to tables. - This PR make the `rustc_smir` mod public.

celinval

Looks good! Thanks.

compiler/rustc_smir/src/stable_mir/compiler_interface.rs

compiler/rustc_smir/src/rustc_internal/mod.rs

Co-authored-by: Celina G. Val <[email protected]>

celinval · 2025-04-23T20:29:16Z

@bors r+ rollup

bors · 2025-04-23T20:29:18Z

📌 Commit 5b390cd has been approved by celinval

It is now in the queue for this repository.

StableMIR: Implement `CompilerInterface` This PR implements part of [the document](https://hackmd.io/`@celinaval/H1lJBGse0).` With `TablesWrapper` wrapped by `CompilerInterface`, the stable-mir's TLV stores a pointer to `CompilerInterface`, while the rustc-specific TLV stores a pointer to tables.

Rollup of 23 pull requests Successful merges: - rust-lang#139261 (mitigate MSVC alignment issue on x86-32) - rust-lang#139307 (std: Add performance warnings to HashMap::get_disjoint_mut) - rust-lang#139700 (Autodiff flags) - rust-lang#139752 (set subsections_via_symbols for ld64 helper sections) - rust-lang#139809 (Don't warn about `v128` in wasm ABI transition) - rust-lang#139852 (StableMIR: Implement `CompilerInterface`) - rust-lang#139945 (Extend HIR to track the source and syntax of a lifetime) - rust-lang#140028 (`deref_patterns`: support string and byte string literals in explicit `deref!("...")` patterns) - rust-lang#140139 (rustc_target: Adjust RISC-V feature implication) - rust-lang#140143 (Move `sys::pal::os::Env` into `sys::env`) - rust-lang#140148 (CI: use aws codebuild for job dist-arm-linux) - rust-lang#140150 (fix MAX_EXP and MIN_EXP docs) - rust-lang#140172 (Make algebraic functions into `const fn` items.) - rust-lang#140177 ([compiletest] Parallelize test discovery) - rust-lang#140181 (Remove `synstructure::Structure::underscore_const` calls.) - rust-lang#140184 (Update doc of cygwin target) - rust-lang#140186 (Rename `compute_x` methods) - rust-lang#140187 ([AIX] Handle AIX dynamic library extensions within c-link-to-rust-dylib run-make test) - rust-lang#140191 (Remove git repository from git config) - rust-lang#140194 (minicore: Have `//@ add-core-stubs` also imply `-Cforce-unwind-tables=yes`) - rust-lang#140195 (triagebot: label minicore changes w/ `A-test-infra-minicore` and ping jieyouxu on changes) - rust-lang#140202 (Make #![feature(let_chains)] bootstrap conditional in compiler/) - rust-lang#140214 (Remove comment about handling non-global where bounds with corresponding projection) r? `@ghost` `@rustbot` modify labels: rollup

StableMIR: Implement `CompilerInterface` This PR implements part of [the document](https://hackmd.io/``@celinaval/H1lJBGse0).`` With `TablesWrapper` wrapped by `CompilerInterface`, the stable-mir's TLV stores a pointer to `CompilerInterface`, while the rustc-specific TLV stores a pointer to tables.

rustbot assigned scottmcm Apr 15, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 15, 2025

makai410 force-pushed the smir-refactor branch from a166ad1 to acec83d Compare April 15, 2025 08:05

rustbot assigned celinval and unassigned scottmcm Apr 15, 2025

makai410 commented Apr 15, 2025

View reviewed changes

celinval reviewed Apr 15, 2025

View reviewed changes

celinval reviewed Apr 18, 2025

View reviewed changes

makai410 force-pushed the smir-refactor branch from 76f2bdd to e4df70e Compare April 18, 2025 06:16

Implement SmirInterface

35a20de

- With `Context` wrapped by `SmirInterface`, the stable-mir's TLV stores a pointer to `SmirInterface`, while the rustc-specific TLV stores a pointer to tables. - This PR make the `rustc_smir` mod public.

makai410 force-pushed the smir-refactor branch from e480dcd to 35a20de Compare April 18, 2025 06:34

celinval approved these changes Apr 21, 2025

View reviewed changes

compiler/rustc_smir/src/stable_mir/compiler_interface.rs Outdated Show resolved Hide resolved

compiler/rustc_smir/src/rustc_internal/mod.rs Outdated Show resolved Hide resolved

Make SmirInterface pub(crate) and rename Context to SmirContext

5b390cd

Co-authored-by: Celina G. Val <[email protected]>

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 23, 2025

Zalathar mentioned this pull request Apr 24, 2025

Rollup of 23 pull requests #140231

Open

matthiaskrgr mentioned this pull request Apr 24, 2025

Rollup of 9 pull requests #140239

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

StableMIR: Implement `CompilerInterface` #139852

StableMIR: Implement `CompilerInterface` #139852

makai410 commented Apr 15, 2025 •

edited

Loading

rustbot commented Apr 15, 2025

makai410 commented Apr 15, 2025

makai410 Apr 15, 2025

celinval left a comment

celinval Apr 15, 2025

makai410 Apr 16, 2025

makai410 Apr 16, 2025

makai410 Apr 16, 2025 •

edited

Loading

celinval Apr 15, 2025

makai410 Apr 15, 2025

makai410 Apr 16, 2025 •

edited

Loading

celinval Apr 16, 2025

celinval Apr 16, 2025

makai410 Apr 16, 2025

celinval Apr 15, 2025

celinval Apr 18, 2025

makai410 Apr 18, 2025

celinval left a comment

celinval commented Apr 23, 2025

bors commented Apr 23, 2025

StableMIR: Implement CompilerInterface #139852

Are you sure you want to change the base?

StableMIR: Implement CompilerInterface #139852

Conversation

makai410 commented Apr 15, 2025 • edited Loading

rustbot commented Apr 15, 2025

makai410 commented Apr 15, 2025

Choose a reason for hiding this comment

celinval left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

makai410 Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

makai410 Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

celinval left a comment

Choose a reason for hiding this comment

celinval commented Apr 23, 2025

bors commented Apr 23, 2025

StableMIR: Implement `CompilerInterface` #139852

StableMIR: Implement `CompilerInterface` #139852

makai410 commented Apr 15, 2025 •

edited

Loading

makai410 Apr 16, 2025 •

edited

Loading

makai410 Apr 16, 2025 •

edited

Loading